src/main/java/com/example/ManyParametersBuilder.java (1)

15-18: Consider validation for timeout and size parameters.

The builder accepts any integer value for timeout and size, including negative values. Depending on your domain model, negative values may not be semantically valid.

If negative values should be rejected, add validation:

 public ManyParametersBuilder setTimeout(int timeout) {
+    if (timeout < 0) {
+        throw new IllegalArgumentException("timeout cannot be negative");
+    }
     this.timeout = timeout;
     return this;
 }

 public ManyParametersBuilder setSize(int size) {
+    if (size < 0) {
+        throw new IllegalArgumentException("size cannot be negative");
+    }
     this.size = size;
     return this;
 }

Also applies to: 25-28

src/main/resources/css/style.css (1)

9-9: Add missing semicolon for consistency.

Line 9 is missing a semicolon after the property value. While this may work in some parsers, it's best practice to include it for consistency.

Apply this diff:

-    -fx-stroke-width: 1px
+    -fx-stroke-width: 1px;

src/main/java/com/example/HelloController.java (2)

38-42: Replace Swedish comments with English.

The code contains Swedish comments (lines 38, 42). For international collaboration and maintainability, please use English for all code comments.

Apply this diff:

-        // Modellens API kräver setMessageToSend() + sendMessage()
+        // Model API requires setMessageToSend() + sendMessage()
         model.setMessageToSend(text);
         model.sendMessage();
 
-        // Rensa input-fältet (matchar även testförväntningarna)
+        // Clear input field (matches test expectations)
         inputMessage.clear();

---

34-53: Consider adding error handling for send operations.

Both sendMessage() and attachFile() methods lack error handling. If the network request fails or file reading fails, the user receives no feedback. Consider wrapping these operations in try-catch blocks and displaying error messages to the user.

src/main/java/com/example/Singelton.java (1)

3-14: Remove the unused Singelton class to keep the codebase clean.

Verification confirms the class has no external usages—all occurrences are internal to the class definition itself (constructor, static initializer, return type). This class should be removed.

src/main/java/com/example/NtfyConnection.java (1)

15-20: Consider adding lifecycle management methods.

The receive() method appears to establish a long-lived connection, but there's no corresponding method to stop receiving or close the connection. Consider adding a close() or stop() method to properly manage resources.

src/main/java/com/example/NtfyConnectionImpl.java (2)

62-62: Remove debug statement from production code.

The peek(System.out::println) should be removed or replaced with proper logging.

---

16-27: Add input validation and consider HTTP client configuration.

Consider the following improvements:

1. Validate hostName is not null/empty in the constructor
2. Configure timeouts on the HttpClient to prevent indefinite hangs
3. Consider making the HttpClient configurable for testing

Example configuration:

private final HttpClient http = HttpClient.newBuilder()
        .connectTimeout(Duration.ofSeconds(10))
        .build();

public NtfyConnectionImpl(String hostName) {
    if (hostName == null || hostName.isBlank()) {
        throw new IllegalArgumentException("hostName must not be null or blank");
    }
    this.hostName = hostName;
}

src/main/java/com/example/HelloModel.java (2)

56-63: Improve error handling in sendFile.

Simply calling e.printStackTrace() provides poor error visibility and user experience. Consider:

1. Logging the error properly
2. Showing user-friendly error notification via the UI
3. Returning or throwing to allow the caller to handle the error

---

45-52: Consider validating before clearing messageToSend.

Currently, messageToSend is always cleared even if the message wasn't sent (when it's blank). While this may be intentional, consider whether you want to preserve the field value if the send is skipped.

Example alternative:

 public void sendMessage() {
     if (messageToSend != null && !messageToSend.isBlank()) {
         connection.send(messageToSend);
+        messageToSend = "";
     }
-
-    // Fältet ska tömmas efter försök att skicka
-    messageToSend = "";
 }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/main/java/com/example/HelloModel.java (2)

60-62: Prevent multiple listener registrations.

Since receiveMessages() is public, it can be invoked multiple times, potentially registering duplicate listeners with connection.receive(...). This could result in messages being added to the list multiple times.

Add a guard to ensure the listener is registered only once:

+private boolean listeningStarted = false;
+
 public void receiveMessages() {
+    if (listeningStarted) {
+        return;
+    }
+    listeningStarted = true;
     connection.receive(m -> Platform.runLater(() -> messages.add(m)));
 }

Alternatively, redesign NtfyConnection.receive() to be idempotent or to return a subscription handle that can be checked.

---

41-46: Consider clearing messageToSend only after successful send.

Currently, messageToSend is cleared unconditionally (line 45), even when the message is null or blank and not sent. While this may be intentional to always reset the input field, it might be clearer to clear only after a successful send.

If you prefer to clear only on success:

 public void sendMessage() {
     if (messageToSend != null && !messageToSend.isBlank()) {
         connection.send(messageToSend);
+        messageToSend = "";
     }
-    messageToSend = "";
 }

Otherwise, document the current behavior (always clearing) to clarify the design intent.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/main/java/com/example/NtfyConnectionImpl.java (4)

33-33: Fix incorrect Cache header.

The header "Cache": "no" is non-standard. Use "Cache-Control": "no-cache" or "Cache-Control": "no-store" instead.

Apply this diff:

-                .header("Cache", "no")
+                .header("Cache-Control", "no-cache")

---

37-43: Blocking I/O will freeze the JavaFX UI thread.

The synchronous http.send() call blocks until the HTTP request completes. If called from a JavaFX thread, this will freeze the UI. Additionally, catching InterruptedException without restoring the interrupt status is incorrect.

Apply this diff to use async and restore interrupt status:

-        try {
-            http.send(httpRequest, HttpResponse.BodyHandlers.discarding());
-            return true;
-        } catch (IOException | InterruptedException e) {
-            e.printStackTrace();
-            return false;
-        }
+        try {
+            http.sendAsync(httpRequest, HttpResponse.BodyHandlers.discarding())
+                .thenApply(response -> true)
+                .exceptionally(e -> {
+                    System.err.println("Failed to send message: " + e.getMessage());
+                    return false;
+                });
+            return true;
+        } catch (Exception e) {
+            System.err.println("Failed to send message: " + e.getMessage());
+            return false;
+        }

Or change the return type to CompletableFuture<Boolean> to properly handle async results.

---

53-69: Blocking I/O will freeze the JavaFX UI thread.

The synchronous http.send() call on line 54 blocks until the HTTP stream completes. This is particularly problematic for the receive() method, which appears designed for long-polling. If called from a JavaFX thread, this will freeze the UI indefinitely. Additionally, InterruptedException should restore the interrupt status.

Replace with sendAsync() and handle the stream asynchronously:

-        try {
-            http.send(httpRequest, HttpResponse.BodyHandlers.ofLines())
-                    .body()
+        http.sendAsync(httpRequest, HttpResponse.BodyHandlers.ofLines())
+                .thenAccept(response -> response.body()
                     .map(s -> {
                         try {
                             return mapper.readValue(s, NtfyMessageDto.class);
                         } catch (IOException e) {
                             return null;
                         }
                     })
                     .filter(Objects::nonNull)
                     .filter(message -> "message".equals(message.event()))
-                    .forEach(messageHandler);
-
-        } catch (IOException | InterruptedException e) {
-            e.printStackTrace();
-        }
+                    .forEach(messageHandler))
+                .exceptionally(e -> {
+                    System.err.println("Failed to receive messages: " + e.getMessage());
+                    return null;
+                });

---

81-85: Blocking I/O will freeze the JavaFX UI thread.

The synchronous http.send() call blocks until the file upload completes. For large files, this will freeze the UI. Additionally, InterruptedException should restore the interrupt status.

Replace with sendAsync():

-        try {
-            http.send(request, HttpResponse.BodyHandlers.discarding());
-        } catch (IOException | InterruptedException e) {
-            e.printStackTrace();
-        }
+        http.sendAsync(request, HttpResponse.BodyHandlers.discarding())
+                .thenAccept(response -> System.out.println("File sent: " + filename))
+                .exceptionally(e -> {
+                    System.err.println("Failed to send file: " + e.getMessage());
+                    return null;
+                });

src/main/java/com/example/HelloModel.java (2)

18-24: Don't call receiveMessages() in the constructor.

Invoking receiveMessages() before the constructor completes allows the this reference to escape. If connection.receive(...) starts an asynchronous operation or background thread, the callback may access a partially constructed HelloModel instance, leading to concurrency hazards.

Move the call outside the constructor:

 public HelloModel(NtfyConnection connection) {
     this.connection = connection;
     messages.add(new NtfyMessageDto("init", 0, "message", "mytopic", "Initial message"));
-    receiveMessages();
 }

Then, ensure the caller (e.g., HelloController) explicitly invokes receiveMessages() after constructing the model:

HelloModel model = new HelloModel(connection);
model.receiveMessages();

---

45-52: Add null check, handle large files, and use proper logging.

This method has several issues:

1. Missing null check: No validation for the file parameter, risking NPE.
2. Memory issue: Files.readAllBytes() loads the entire file into memory, which can cause OutOfMemoryError for large files.
3. Poor error handling: printStackTrace() is an anti-pattern; use a logging framework instead.

Apply this diff:

 public void sendFile(File file) {
+    if (file == null || !file.exists() || !file.canRead()) {
+        System.err.println("Invalid file: " + (file != null ? file.getName() : "null"));
+        return;
+    }
     try {
         byte[] data = Files.readAllBytes(file.toPath());
         connection.sendFile(file.getName(), data);
     } catch (IOException e) {
-        e.printStackTrace();
+        System.err.println("Failed to send file: " + e.getMessage());
     }
 }

For large files, consider streaming the data instead of reading it all at once, if NtfyConnection.sendFile supports streaming APIs, or check file.length() and refuse files above a safe threshold (e.g., 10 MB).

src/main/java/com/example/NtfyConnectionImpl.java (1)

14-99: Replace printStackTrace() with proper logging.

Throughout the class (lines 41, 60, 67, 84, 97), errors are handled with printStackTrace(). In production code, use a logging framework (e.g., SLF4J, java.util.logging) for consistent error handling and log management.

Add a logger field:

private static final Logger logger = Logger.getLogger(NtfyConnectionImpl.class.getName());

Then replace e.printStackTrace() with:

logger.log(Level.SEVERE, "Error message", e);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro